Skip to content

feat(commands): defineCommand — typed declarative commands via an ICommand adapter - #6101

Merged
NathanWalker merged 9 commits into
mainfrom
feat/define-command
Aug 6, 2026
Merged

feat(commands): defineCommand — typed declarative commands via an ICommand adapter#6101
NathanWalker merged 9 commits into
mainfrom
feat/define-command

Conversation

@edusperoni

@edusperoni edusperoni commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Based on main#6099 (the DI foundation) is merged.

PR Checklist

What is the current behavior?

A command is a class implementing ICommand, registered under a stringly-typed key, reading flags off the untyped global $options object. The validation semantics carry a trap: declaring canExecute silently disables allowedParameters validation, and an empty allowedParameters means "reject all positional arguments" — none of which the types express.

What is the new behavior?

defineCommand — a declarative, typed command definition that plugs into the existing registry through an adapter (createCommandFromDefinition / registerCommandDefinition, routed through the CommandRegistry facet with a real useFactory provider). Fully additive: routing, help, hooks, and analytics behavior are untouched, and legacy ICommand classes remain fully supported.

export default defineCommand({
	name: "widget|add",
	options: { overwrite: booleanOption({ default: false }), output: stringOption({ alias: "o" }) },
	arguments: "none",
	async run(ctx) {
		// ctx.options.overwrite is boolean (has a default); ctx.options.output is string | undefined
		if (!ctx.options.output) ctx.fail("An output path is required.");
	},
});
  • Honest option types: an option with a default is T; without one it is T | undefined — pinned by a strict-mode compile fixture (test/type-fixtures/), since the repo's own build has strictNullChecks off and any in-suite assertion would be vacuous. A no-options command's ctx.options rejects typos.
  • The canExecute trap is gone: the adapter always enforces the declared arguments policy first, then calls a user canExecute as pure refinement — the fields compose instead of interacting.
  • Define-time validation: bad option types, unknown fields, missing run, invalid aliases all throw at defineCommand() with messages naming the command. Options colliding with CLI-wide option names or aliases get a define-time warning naming both sides.
  • ctx.fail(message) fails the command through failWithHelp; plain throw remains equivalent. Both run and canExecute execute in an injection context, so inject() works inside commands the same as everywhere else.
  • Option schemas compile to dashedOptions, riding the CLI's revived option validation (unknown flags warn by default today and hard-fail under NS_STRICT_OPTIONS=error).
  • lib/common/define-command.ts is side-effect-free and exported from nativescript/contracts; definitions carry a plain-assigned, spread-safe Symbol.for marker; isCommandDefinition is a type predicate and the return type is branded — registerCommandDefinition requires it at compile time and verifies it at runtime.
  • Public type names follow the new-API convention (no I prefix): CommandDefinition, CommandContext, CommandOptionSpec. Legacy published I* types untouched.
  • 46 tests in test/define-command.ts, including end-to-end dispatch through the parent name (tryExecuteCommand("widget", ["add"]) and a *default case) and through CommandsService.tryExecuteCommand.
  • New authoring guide: defining-commands.md.

The registry gaps this work surfaced were fixed on main rather than in this PR: registerCommand now populates hierarchical routing state, a registered command is no longer silently clobbered when a subcommand later shadows it (which also revealed and fixed the CLI's own dead widget flat registration), and options declared with an array of aliases resolve correctly under the revived validator.

Full suite: 115 files, 1759 passed / 9 skipped (main baseline 1713 + 46 branch tests); yok oracle, public-API test, and compat fixtures untouched.

Summary by CodeRabbit

  • New Features

    • Added a declarative API for defining CLI commands with typed options, defaults, aliases, validation, positional arguments, and execution contexts.
    • Added support for availability checks, dependency injection, analytics, hooks, and structured failure handling.
    • Declarative commands can coexist with existing command implementations and be registered for dispatch.
  • Documentation

    • Added comprehensive guidance for defining, configuring, registering, and using declarative CLI commands.
  • Tests

    • Added extensive runtime and compile-time coverage for command definitions, parsing, validation, registration, execution, and error handling.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 76038a66-5441-43f1-aab4-6f8030ddd5f2

📥 Commits

Reviewing files that changed from the base of the PR and between d712a4e and 86b4762.

📒 Files selected for processing (1)
  • lib/contracts/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/contracts/index.ts

📝 Walkthrough

Walkthrough

The change adds a typed declarative command API. It validates and brands definitions, converts them to legacy ICommand objects, supports registration and dispatch, and adds runtime and compile-time coverage.

Changes

Declarative commands

Layer / File(s) Summary
Command contract and public API
lib/common/define-command.ts, lib/contracts/index.ts, test/type-fixtures/*, tsconfig.json, defining-commands.md
Defines typed command options, contexts, validation, branding, public exports, documentation, and compile-time fixtures.
Definition adaptation and registration
lib/common/services/command-definition-adapter.ts
Compiles options, detects CLI collisions, creates legacy commands with injection-aware execution, and registers marked definitions lazily.
Registration, execution, and dispatch validation
test/define-command.ts
Tests registration, aliases, hierarchy, option parsing, execution, canExecute, failures, flags, collisions, and end-to-end dispatch.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CommandsService
  participant CommandRegistry
  participant CommandDefinitionAdapter
  participant OptionsService
  participant CommandContext
  CommandsService->>CommandRegistry: dispatch registered command
  CommandRegistry->>CommandDefinitionAdapter: create command from definition
  CommandDefinitionAdapter->>OptionsService: resolve option values
  CommandDefinitionAdapter->>CommandContext: construct execution context
  CommandDefinitionAdapter->>CommandContext: run canExecute and run
Loading

Possibly related PRs

Suggested reviewers: nathanwalker

Poem

A rabbit typed commands in the morning light,
With options and aliases arranged just right.
The registry hopped, the contexts ran,
Failures spoke clearly, as planned.
“Declarative carrots!” the rabbit cheered.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a typed declarative defineCommand API with an ICommand adapter.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@edusperoni
edusperoni force-pushed the feat/define-command branch 4 times, most recently from cafa737 to a1ba0ef Compare July 30, 2026 02:51
Base automatically changed from feat/di-modernization-phase1 to main August 3, 2026 00:31
@edusperoni
edusperoni force-pushed the feat/define-command branch from a1ba0ef to 07c979c Compare August 4, 2026 20:41
…adapter

Commands can now be declared as plain objects: a name, an option schema
built from booleanOption/stringOption/numberOption/arrayOption, and a run
function whose context carries the positional args plus the declared
options, typed by inference from the schema.

lib/common/define-command holds the types and the pure factories only, so
it stays side-effect-free and can be re-exported from
nativescript/contracts. The runtime bridge lives in
lib/common/services/command-definition-adapter, which compiles a
definition into the ICommand the legacy registry expects and runs it
inside an injection context.

canExecute is emitted only when the definition supplies one or opts into
arguments: "any"; CommandsService skips all parameter validation as soon
as canExecute exists, so omitting it is what lets the framework reject
stray positional arguments for arguments: "none".

Fully additive — existing ICommand classes are untouched.
…nitions

A definition with no declared options must be executable in a container
that has no options service registered - manifest-loaded extension
commands run in exactly that situation.
The parent-dispatcher leak onto the module-level injector is fixed in
the base branch, so the round-trip test no longer needs the global
facade.
Yok extends Injector on the base branch; the di bridge is gone.
…rgument policy

Reworks the declarative command API after the design review:

- the new public types drop the `I` prefix, and `defineCommand` returns a
  `DefinedCommand` branded with the marker `isCommandDefinition` narrows
  to. `registerCommandDefinition` requires that brand, so nothing reaches
  the registry without having been validated.
- an option is `T` only when its spec declares a `default`; without one
  it is `T | undefined`, which is what the command line actually
  produces. Asserted by test/type-fixtures, compiled under strict mode
  because this build has strictNullChecks off.
- `defineCommand` validates the definition and throws naming the command
  and the accepted form, instead of failing deep and unattributed later.
- `arguments` is enforced before the definition's `canExecute` runs, so
  the two compose: a command that leaves `arguments` at "none" rejects
  stray positional arguments whether or not it refines further.
- `canExecute` runs in an injection context, like `run`.
- registration goes through the `CommandRegistry` facet the target
  injector provides rather than the injector itself.
- a schema entry shadowing a CLI-wide option warns naming the collision.
Unknown options warn and only fail under NS_STRICT_OPTIONS=error;
`description` reaches the parser but nothing renders it; `canExecute`
gets a context of the same shape as run's, not the same one. Replaces
the "canExecute owns validation" rule with how the two fields compose,
renames the flagship example's option off the CLI-wide `verbose`, and
documents option value types, array aliases, the parent-name collision
and `satisfies` for shared schemas.
`ctx.fail(message)` is the failure verb on the command context, in both
`run` and `canExecute`. It maps to the errors service's `failWithHelp`,
so a command failure carries the usage suggestion, and returns `never` so
it can end a branch without a return. The message is validated like the
define-time errors are, naming the command. Throwing keeps working
unchanged — fail() is sugar over it, not a replacement. Commands get no
`skip()`: warn-and-continue has no meaning inside run().

The CLI-wide option collision warning now covers aliases on both sides,
so an `alias: "p"` that shadows `--path`'s shorthand is reported the same
way a `verbose` option shadowing `--verbose` is, naming both sides.
@edusperoni
edusperoni force-pushed the feat/define-command branch from 07c979c to d712a4e Compare August 5, 2026 19:22
@edusperoni
edusperoni marked this pull request as ready for review August 5, 2026 20:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
lib/common/services/command-definition-adapter.ts (1)

74-97: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

cliSpellings inherits Object.prototype keys, so some option names report a false collision.

cliSpellings is a plain object literal. An option or alias named constructor, toString, or valueOf resolves through the prototype chain, so line 84 or line 91 is truthy without any real collision. The warning then interpolates a function into the message. The same hardening applies to the options object built on line 163.

Use a prototype-less object for both.

♻️ Proposed change
-	const cliSpellings: IDictionary<string> = {};
+	const cliSpellings: IDictionary<string> = Object.create(null);
 	const buildContext = (args: string[]): CommandContext<TSchema> => {
-		const options: any = {};
+		const options: any = Object.create(null);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/services/command-definition-adapter.ts` around lines 74 - 97, Use
prototype-less objects for both the cliSpellings map in the collision checks and
the options object built later in the command-definition adapter. Preserve their
existing key assignments and lookups while preventing inherited Object.prototype
names from being treated as real options or aliases.
lib/common/define-command.ts (1)

332-340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Validation accepts a definition whose run lives on a prototype; the returned copy drops it.

isPlainObject accepts a class instance, and line 277 finds run through the prototype chain. The spread on line 337 copies own enumerable properties only, so the returned DefinedCommand has no run. The failure then surfaces later, when the command executes, instead of at define time.

Reject a definition whose run is not an own property, or copy the resolved handlers explicitly.

♻️ Proposed check
 	if (typeof definition.run !== "function") {
 		invalid(definition, "'run' must be a function");
 	}
+
+	if (!Object.prototype.hasOwnProperty.call(definition, "run")) {
+		invalid(
+			definition,
+			"'run' must be declared on the definition object itself, not inherited from a prototype",
+		);
+	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/define-command.ts` around lines 332 - 340, Update defineCommand
and its validation flow so definitions with a prototype-inherited run handler
are rejected before the spread copy is returned; require run to be an own
property while preserving valid own-handler definitions and existing validation
behavior.
defining-commands.md (1)

60-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a language to the fenced block.

markdownlint reports MD040 here. The block holds an error message, so mark it as text.

📝 Proposed change
-```
+```text
 Invalid command definition for 'widget|add': unknown field(s) 'handler'; a
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@defining-commands.md` around lines 60 - 66, Update the fenced code block in
defining-commands.md to specify the text language, changing the opening fence to
```text while preserving the error message content.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/common/services/command-definition-adapter.ts`:
- Around line 173-181: Update createCommandFromDefinition to pass
definition.description through in the returned command object when it is
defined, alongside disableAnalytics and enableHooks, so the adapter preserves
the validated command metadata.

---

Nitpick comments:
In `@defining-commands.md`:
- Around line 60-66: Update the fenced code block in defining-commands.md to
specify the text language, changing the opening fence to ```text while
preserving the error message content.

In `@lib/common/define-command.ts`:
- Around line 332-340: Update defineCommand and its validation flow so
definitions with a prototype-inherited run handler are rejected before the
spread copy is returned; require run to be an own property while preserving
valid own-handler definitions and existing validation behavior.

In `@lib/common/services/command-definition-adapter.ts`:
- Around line 74-97: Use prototype-less objects for both the cliSpellings map in
the collision checks and the options object built later in the
command-definition adapter. Preserve their existing key assignments and lookups
while preventing inherited Object.prototype names from being treated as real
options or aliases.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 61c4e27b-40eb-4bf0-a237-8136d760e17f

📥 Commits

Reviewing files that changed from the base of the PR and between 0549b1c and d712a4e.

📒 Files selected for processing (8)
  • defining-commands.md
  • lib/common/define-command.ts
  • lib/common/services/command-definition-adapter.ts
  • lib/contracts/index.ts
  • test/define-command.ts
  • test/type-fixtures/define-command-types.ts
  • test/type-fixtures/tsconfig.json
  • tsconfig.json

Comment on lines +173 to +181
return {
allowedParameters: [],
dashedOptions,
...(definition.disableAnalytics === undefined
? {}
: { disableAnalytics: definition.disableAnalytics }),
...(definition.enableHooks === undefined
? {}
: { enableHooks: definition.enableHooks }),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Determine whether a per-command description can be carried on the command object.
set -uo pipefail

# ICommand / ICommandOptions surface
fd -t f 'commands.d.ts' lib | xargs -r rg -n -C4 'interface ICommand\b|interface ICommandOptions'

# How help resolves per-command text
rg -n -C4 --type=ts 'commandHelp|getCommandHelp|helpCommand|description' lib/common/services/help-service.ts 2>/dev/null

# Any existing consumer of a description on a command object
rg -nP --type=ts -C3 '\bcommand\.description\b|\bdescription\b.*ICommand'

Repository: NativeScript/nativescript-cli

Length of output: 412


🏁 Script executed:

#!/bin/bash
set -uo pipefail

printf 'Repo files matching command-definition-adapter.ts / define-command.ts:\n'
fd -t f 'command-definition-adapter.ts|define-command.ts|defining-commands.md' .

printf '\nCommandDefinitionAdapter adapter relevant section:\n'
f="$(fd -t f 'command-definition-adapter.ts' . | head -n1)"
[ -n "$f" ] && sed -n '130,210p' "$f" | nl -ba -v130

printf '\ndefine-command validation around description:\n'
f="$(fd -t f 'define-command.ts' . | head -n1)"
[ -n "$f" ] && sed -n '280,330p' "$f" | nl -ba -v280

printf '\nhelp service description consumers:\�'
fd -t f 'help-service.ts' lib | xargs -r sed -n '1,260p' | nl -ba | rg -n -C5 'description|commandHelp|getCommandHelp|helpCommand|ICommand'

printf '\nFind descriptions in declarations and commands.d.ts:\n'
fd -t f 'commands.d.ts' lib | xargs -r rg -n -C3 'interface ICommandOptions\b|interface ICommand\b|description'

Repository: NativeScript/nativescript-cli

Length of output: 926


🏁 Script executed:

#!/bin/bash
set -uo pipefail

f_adapter="$(fd -t f 'command-definition-adapter.ts' . | head -n1)"
f_define="$(fd -t f 'define-command.ts' . | head -n1)"
f_help="$(fd -t f 'help-service.ts' lib | head -n1)"
f_decl="$(fd -t f 'commands.d.ts' lib | head -n1)"

printf 'adapter %s validation and transform:\n' "$f_adapter"
sed -n '130,210p' "$f_adapter"

printf '\ndefine validation %s:\n' "$f_define"
sed -n '260,335p' "$f_define"

printf '\nhelp service relevant description/help consumers %s:\n' "$f_help"
sed -n '1,240p' "$f_help" | grep -n -C 5 'description\|commandHelp\|getCommandHelp\|helpCommand\|ICommand' || true

printf '\ndeclarations %s:\n' "$f_decl"
sed -n '1,80p' "$f_decl"

printf '\nAny TypeScript/d declaration consumers for command.description:\n'
rg -n -C 3 --iglob '*.{ts,d.ts}' '\\bcommand\.description\b|ICommandOptions' .

Repository: NativeScript/nativescript-cli

Length of output: 8973


🏁 Script executed:

#!/bin/bash
set -uo pipefail

printf 'DefineCommand and CommandDefinition type definitions:\n'
rg -n -C8 -P 'type DefinedCommand|interface (CommandDefinition|ICommandOptions)\b|description' lib/common/define-command.ts lib/common/declarations.d.ts

printf '\nHelpService full relevant methods around reading command files:\n'
f="$(fd -t f 'help-service.ts' lib | head -n1)"
sed -n '80,220p' "$f"

printf '\nCommandData interface and construction references:\n'
rg -n -C4 -P 'interface ICommandData\b|type ICommandData\b|showCommandLineHelp|openHelpForCommandInBrowser|ICommandData' lib test | head -n 240

printf '\ncommand-definition-adapter usages and validateDefinition references:\n'
rg -n -C3 'registerCommandDefinition|defineCommand|validateDefinition|definition\.description|DEFINITION_FIELDS' lib test

Repository: NativeScript/nativescript-cli

Length of output: 50385


🏁 Script executed:

#!/bin/bash
set -uo pipefail

rg -n --iglob '*.{md,d.ts,ts}' -C3 '\binterface ICommandData\b|\btype ICommandData\b|description' test/define-command.ts test || true

printf '\nCheck help markup references in generated command descriptions:\n'
rg -n --iglob '*.{md,d.ts,ts}' -C2 'Generated from command\\.|commandDescription|module\.commandDescription|#{module\.command' .

printf '\nCheck exact help service methods that read command docs:\n'
f="$(fd -t f 'help-service.ts' lib | head -n1)"
sed -n '220,340p' "$f"

printf '\nRead command-definition-adapter mapping section using Python line numbers:\n'
python3 - <<'PY'
from pathlib import Path
for p in Path('.').rglob('command-definition-adapter.ts'):
    print(f'\n{p}')
    text = p.read_text()
    lines = text.splitlines()
    for i in range(max(0, len(lines)-1), -1, -1):
        if 'function createCommandFromDefinition' in lines[i]:
            start = max(0, i-10); end = min(len(lines), i+90)
            break
    else:
        continue
    for n, line in enumerate(lines[start:end], start=start+1):
        print(f'{n:4d}\t{line}')
PY

Repository: NativeScript/nativescript-cli

Length of output: 19670


Pass definition.description through the command adapter.

CommandDefinition validates and documents description, but createCommandFromDefinition only copies disableAnalytics and enableHooks, so the registered command object drops it. Map description: definition.description when present, or document the field as non-passed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/services/command-definition-adapter.ts` around lines 173 - 181,
Update createCommandFromDefinition to pass definition.description through in the
returned command object when it is defined, alongside disableAnalytics and
enableHooks, so the adapter preserves the validated command metadata.

@NathanWalker

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

# Conflicts:
#	lib/contracts/index.ts

Co-authored-by: NathanWalker <[email protected]>

Copilot AI commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main and fixing the conflict in lib/contracts/index.ts so both command and hook exports are preserved. Commit: 86b4762d.

@NathanWalker
NathanWalker merged commit 2f81d7f into main Aug 6, 2026
1 check passed
@NathanWalker
NathanWalker deleted the feat/define-command branch August 6, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants